Conversation
Pixels are now skipped when they have no amplitude, i.e. when the standard deviation equals 0.
|
@JulietteFrancovich I think this is what we discussed. For me, this fixes the issue where some/a lot of pixels do not show up on the TIV map. We should discuss which definition we use when creating TIV maps. I think there are two options:
What do you think? |
|
As discussed, I updated the code.
|
|
@JulietteFrancovich I updated the code. Is there a way for you to test it with some of your existing processes, so that we can properly test whether the DeprecationWarning works as intended? |
| if self.allow_negative_amplitude and mean_tiv < 0: | ||
| start_func, middle_func = np.argmax, np.argmin | ||
| lagged_indices_breath_middles = indices_breath_middles | ||
| else: | ||
| start_func, middle_func = np.argmin, np.argmax | ||
|
|
||
| cd = np.copy(continuous_data.values) | ||
| cd -= np.nanmean(cd) | ||
| pi = np.copy(pixel_impedance[:, row, col]) | ||
| pi -= np.nanmean(pixel_impedance[:, row, col]) | ||
|
|
||
| if self.correct_for_phase_shift: | ||
| # search for maximum cross correlation within MAX_XCORR_LAG times the average | ||
| # duration of a breath | ||
| xcorr = signal.correlate(cd, pi, mode="same") | ||
| max_lag = MAX_XCORR_LAG * np.mean(np.diff(indices_breath_middles)) | ||
| lag_range = (lags > -max_lag) & (lags < max_lag) | ||
| # TODO: if this does not work, implement robust peak detection | ||
| lag = lags[lag_range][np.argmax(xcorr[lag_range])] | ||
| # positive lag: pixel inflates later than summed | ||
|
|
||
| # shift search area | ||
| lagged_indices_breath_middles = indices_breath_middles - lag | ||
| lagged_indices_breath_middles = lagged_indices_breath_middles[ | ||
| (lagged_indices_breath_middles >= 0) & (lagged_indices_breath_middles < len(cd)) | ||
| ] |
There was a problem hiding this comment.
does it make sense to have allow_negative_amplitude and correct_phase_shift as separate arguments? Allowing negative amplitude without phase shift correction may lead to misinterpretation of out-of-phase signals (as this is the reason for implementing this, right?)
There was a problem hiding this comment.
No, I think you're correct. I was struggling with that last night but didn't come up with a good solution. I think it's better to have a some sort of 'phase_correction_mode' argument with three options, resulting in setting internal boolean variables:
- "negative amplitude" ->
allow_negative_amplidude = True(default) - "correct phase shift" ->
allow_negative_amplitude = False,correct_phase_shift=True - "none"/
None->allow_negative_amplitude = False,correct_phase_shift=False
Would that make sense? Do you have better suggestions for the mode names?
There was a problem hiding this comment.
Re-reading your question: yes, I do think it makes sense to have the 'none' option above. This is the old behaviour, and
it works in at least some cases. If it's not necessary to overcomplicate, why should we?
There was a problem hiding this comment.
Yes to me it does make sense to have the none option but I think there shouldn't be an option to have allow_negative_amplitude = True with correct_phase_shift=False, right?
There was a problem hiding this comment.
There isn't an option where allow_negative_amplitude = True with correct_phase_shift=False, because correct_phase_shift is only used when allow_negative_amplitude == False. I have replaced the boolean arguments with a mode argument.
Can you check whether their function is clear from the documentation?
2 participants
This should fix the issue where pixels that have a negative absolute value are ignored during pixel breath detection, resulting in them being excluded in TIV maps.
It also adds a boolean to choose to allow pixels with negative amplitude. There are now two options:
allow_negative_amplitude = True).allow_negative_amplitude = False).Which definition you use will depend on the use case.